Fix several bugs for enabling Paddle to train with CINN. #36739
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your contribution! |
thisjiang
reviewed
Oct 27, 2021
| set_cinn_flag(False) | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() | ||
| do_test(tempfile.mkdtemp(prefix="dots_")) |
thisjiang
reviewed
Oct 27, 2021
| @@ -136,6 +153,18 @@ std::unique_ptr<Graph> CreateNewSubGraph(const GraphNodeSet& cluster, | |||
| } | |||
| old_var2new_var[var] = sub_node; | |||
| } | |||
| for (auto* var : cluster_inputs) { | |||
| if (var->Var()) { | |||
| auto* sub_node = subgraph->CreateVarNode(var->Var()); | |||
There was a problem hiding this comment.
这儿为啥不要var->Var() == nullptr时CreateEmptyNode啊？
There was a problem hiding this comment.
因为子图不需要这种var_desc为null的节点，CINN编译的时候也用不到。而这种空var只是为了加上依赖关系，在修改后的大图上已经有提现。
thisjiang
reviewed
Oct 28, 2021
|
|
||
| def test_run_with_cinn(self): | ||
| do_test(self.tmpdir) | ||
| set_cinn_flag(False) |
There was a problem hiding this comment.
这儿set_cinn_flag(False)应该删了，不然后面的测试都不会运行了
thisjiang
reviewed
Oct 28, 2021
| exe.run(startup_program) | ||
|
|
||
| build_strategy = paddle.static.BuildStrategy() | ||
| build_strategy.debug_graphviz_path = os.path.join(dot_save_dir, "viz") |
There was a problem hiding this comment.
调试信息还是保留吧，这个类会处理这些保存的文件，退出时即删除。
thisjiang
reviewed
Oct 28, 2021
| @@ -130,12 +147,25 @@ std::unique_ptr<Graph> CreateNewSubGraph(const GraphNodeSet& cluster, | |||
| for (auto* var : cluster_internals) { | |||
| Node* sub_node; | |||
| if (var->Var() == nullptr) { | |||
| // TODO(wzzju): If this case occurs, there maybe bugs when using CINN. | |||
There was a problem hiding this comment.
这个bug还在么？如果还在是不是最好弄个PADDLE_ENFORCE？
wzzju
changed the title
wzzju
changed the title
ghost
pushed a commit
to piotrekobi/Paddle
that referenced
this pull request
Nov 3, 2021
…e#36739) * Update the content of `test_parallel_executor_run_cinn.py`. * Fix some bugs in the topological sort and `CreateNewSubGraph`. * Update the CINN commit id used by Paddle. * Update the unit test to `add+relu`. * Update according to reviewers' suggestion.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR types
Bug fixes
PR changes
Others
Describe
#36739 （本PR） 和 #36698 均用于解决Paddle训练接入CINN全流程打通过程中遇到的问题。所有问题列举如下：
build_cinn_pass中找到并创建的子图中可能存在ControlVar，这些ControlVar的VarDesc为NULL，需要对这种corner case进行特殊处理，方案详见 Fix the null ptr bug in build_cinn_pass. #36698 。build_cinn_pass放在ParallelExecutorPassBuilder的最后加入，会与后面的显存优化Pass功能进行冲突，为了避免与其他Pass功能冲突，将build_cinn_pass提前至第一个Pass运行。build_cinn_pass在创建CinnLaunchOp后，需要为其设置op_role属性值，否则在其之后运行的Pass会报错找不到改属性。build_cinn_pass的子图创建逻辑，处理输出节点亦可作为子图内部输入的情况。build_cinn_pass节点之间连接关系，以及无用节点的删除逻辑。add + relu且固定数值的模型运算逻辑（方便精度对齐），并添加调试所需的信息。